Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement RingCT Staking #1019

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Implement RingCT Staking #1019

wants to merge 25 commits into from

Conversation

Zannick
Copy link
Collaborator

@Zannick Zannick commented Oct 30, 2022

Summary

This PR implements the functionality necessary for the creation and validation of blocks via Proof of Stake using unspent RingCT outputs, as described in #1015 and taking much inspiration (and code) from #678.

This does not yet enable RingCT staking (by placing the default start at block 50 million).

Details

Potential RingCT inputs are divided into brackets; the stake weight of a coin corresponds to the minimum value of its bracket, with some higher levels further reduced, similar to zerocoin (see #1015 for details). A RingCT output becomes eligible for staking after 1000 blocks, same as zerocoin.

A RingCT stake block always has two key transactions, the coinbase and the coinstake. The coinbase contains either an empty standard output (most blocks), or the standard outputs from the superblock, plus two RingCT outputs which provably sum to the block stake reward. The coinstake contains a single RingCT input (the staking coin), and three outputs: an empty standard output indicating that it's a coinstake, a data output, and a RingCT output of the same value as the input. The rangeproof for this transaction proves that the input is large enough to pass the stake difficulty, by providing a min_value equal to the minimum value of a bracket. For additional privacy, this is not the bracket the coin is in but the lowest possible bracket that still passes the stake difficulty.

The block is signed using the same key as the staking coin.

Other changes

  • The wallet automatically creates a new stealth address for RingCT staking.
  • Fixes an issue where RingCT marked the wrong thing as spent1, and also adds a way to reset the pending spend status for stakes that fail after the transaction is created.
  • The base_uint class now has a safeMultiply function that informs the caller whether the operation overflowed the datatype.
  • Fixes some small oversights like GetRangeProofInfo returning an int 0 or 1 when it could not be confusing and just return a bool, some error() calls with too many arguments, and a lack of default values for rangeproof parameter overrides.
  • A lot of refactoring, many for clarity.

Potential improvements (for the project as a whole)

Cache the pubkey used to mark the RingCT stake uniqueness (#678 does this), since it may be repetitive across stake attempts to otherwise access it.

Remove the brackets and use a flatter scale with some high cutoff like 1M, given that we can reduce the rangeproof min_value anyway.

Add separate config settings for staking via Zerocoin or RingCT.

Add UI elements for the GUI wallet.

Add new tests/fix existing tests. :)

What next?

This PR needs a thorough review, including a good security review that it won't be possible to generate free coins, and testing in dev. We can discuss further features as well. Finally, we have to pick a height at which to enable on testnet (causing a fork).

Footnotes

  1. Possibly I should split this out for a separate review, though I'm not sure what it affects.

Moves some functionality for creating RingCT transactions into helper
functions.

Removes a duplicate error for when setting a fee of 0 fails. (Later we will
attempt to set a fee of nFeeRet, which would be 0 in this case anyway.)
Change CreateTxOuts to create the actual TxOut pointers
rather than fill a list of TxOuts and separately extract the pointers.
This sets up a basic but incomplete implementation of CreateRingCTStake.

Includes a weighting function that groups coins into buckets along
powers of 16 sats, where the weight of the coin would be equal to the
smallest value in the bucket regardless of the actual value of the coin.
Refactor out the necessary vectors to store separately in the stakeinput,
since we need them from both the input blinds and the output blinds.
And allow static allocation of the context structs.
Move COutputR to a common location.
Hold a copy of COutputR in RingCTStake for its lifetime.
Handle other memory lifetime issues.
Add a helper class for resetting pending spends.
and unmark pending spends if the block is not created.
- Provide the txCoinbase to the CoinStake object for filling out
further if necessary. (Originally, so that we could reference them in
the coinstake, but this won't be necessary if we make them ringct to
begin with. Still, it's easier to have in one place.)
- Fix two misc error() calls that were broken.
- Use blind_sum and verify_tally to confirm that the anon coin rewards
  sum to the expected number.
Remove some debugging statements and other cleanup.
- Stake uniqueness is the hash of the public key of the input.
- Stake indexfrom is the block index at the minimum stake depth.
As the stake weight controls the hash difficulty ceiling, we can simply
choose the lowest bracket in which the stake hash comes under the
ceiling, and use that as our rangeproof min_value for added privacy.

- Fix overflow detection when multiplying arith_uint256.
  Otherwise regtest stake targets can be a little wonky.
- Remove an unnecessary back-and-forth cast to uint256.
- The RingCT stake bracket weights are basically constants, so do that.
Add enable_wallet guards to similar places as existing ones.
Remove unused stake functions from early attempts.
@Zannick Zannick added Component: Wallet Relating to keystore, tx creation, and balance tracking Tag: TransactionRecords The display of transaction information Component: Consensus Part of the core cryptocurrency consensus protocol Tag: PoS Related to Proof of Stake Component: Core App Related to the application itself. Coin Type: RingCT Specifically related to RingCT transactions Component: Miner Both PoW and PoS block creation labels Oct 30, 2022
@Zannick Zannick added Tag: Transaction Creation Related to the transaction creation process. Tag: Waiting For QA A pull review is waiting for QA to test the pull request Tag: Waiting For Code Review Waiting for code review from a core developer Tag: Validation Related to the validation of blocks and transactions. labels Oct 30, 2022
@Zannick Zannick self-assigned this Oct 30, 2022
Copy link
Contributor

@us77ipis us77ipis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just quickly skimmed through the code and found these...

src/validation.cpp Outdated Show resolved Hide resolved
src/validation.cpp Outdated Show resolved Hide resolved
@seanPhill seanPhill added this to the v2.0.0 milestone Dec 21, 2022
@Zannick
Copy link
Collaborator Author

Zannick commented Jan 14, 2023

devnet will start ringct staking alongside zerocoin staking at height 25000.

@Zannick
Copy link
Collaborator Author

Zannick commented Jan 20, 2023

DevNet mining is underway. Sean notes:

In the GUI the RingCT stakes are incorrectly labeled as "Mined on 3txz...longStealthAddress". It's easy to identify them with listtransactions.

We should update those transactions to be RingCT stakes. The transactions themselves will look similar to mining transactions; we'll have to look at the stake transaction next to it.

@seanPhill
Copy link
Collaborator

seanPhill commented Jan 20, 2023

DevNet mining is underway. Sean notes:

In the GUI the RingCT stakes are incorrectly labeled as "Mined on 3txz...longStealthAddress". It's easy to identify them with listtransactions.

We should update those transactions to be RingCT stakes. The transactions themselves will look similar to mining transactions; we'll have to look at the stake transaction next to it.

A second case in the display of (RingCT stake winning) transactions in the GUI is that (maybe if the winning RingCT utxo is seen by the wallet first) it displays as "Zerocoin Stake", but shows the value of the RingCT utxo. Users would be confused as their wallet will say they gained 18001.99083067 Veil, (magically) for example, but their balance will of course have only gained the amount of the block reward (50 Veil on devnet, and always ten Veil on mainnet).

Additional note: These entries in the GUI list where the size of the RingCT winning utxo is shown (as "Zerocoin Stake") are actually matched by the other GUI entry for "Mined on 3tXz..." 50 VEIL (or as it would be on mainnet, 10 Veil.

@seanPhill
Copy link
Collaborator

seanPhill commented Jan 20, 2023

While syncing or resyncing we might need to look into this error too. (Much repeated!)

ERROR: ProcessNewBlock: AcceptBlock FAILED (bad-block-sig, PoS block signature not valid (code 16))(-1)

The above is happening on a peer trying to sync, while the below is happening on the leading wallet, even though this block has already been built on top of and is over fifty blocks confirmed.

Console:

getblockhash 25254
4bbbedcd972c8372e4810ca2af4420071713424392a2bfc8478563411d43a073

debug.log: (much repeated)


2023-01-20T20:25:57Z Reject block code 16: bad-block-sig: hash 4bbbedcd972c8372e4810ca2af4420071713424392a2bfc8478563411d43a073
2023-01-20T20:25:57Z received: getdata (37 bytes) peer=8
2023-01-20T20:25:57Z received getdata (1 invsz) peer=8
2023-01-20T20:25:57Z received getdata for: witness-block 4bbbedcd972c8372e4810ca2af4420071713424392a2bfc8478563411d43a073 peer=8
2023-01-20T20:25:57Z sending block (10309 bytes) peer=8
2023-01-20T20:25:57Z received: reject (53 bytes) peer=8
2023-01-20T20:25:57Z Reject block code 16: bad-block-sig: hash 4bbbedcd972c8372e4810ca2af4420071713424392a2bfc8478563411d43a073
2023-01-20T20:25:57Z received: getdata (37 bytes) peer=8
2023-01-20T20:25:57Z received getdata (1 invsz) peer=8
2023-01-20T20:25:57Z received getdata for: witness-block 4bbbedcd972c8372e4810ca2af4420071713424392a2bfc8478563411d43a073 peer=8
2023-01-20T20:25:57Z sending block (10309 bytes) peer=8
2023-01-20T20:25:57Z received: reject (53 bytes) peer=8

As I don't think any other peer has got past that block, I think I will invalidate it.

(Even after invalidatingblock and a restart, the "leading" wallet refuses to let go of the subsequently staked blocks, while any reconsidering of the block results in it being rejected because of a "bod-block-sig".)
Will try the snapshot again, and use genoverride=1 when ready. (Didn't get to wait to use genoverride, as the node, like others goes off and generates RingCT-staked blocks that duplicate the block height of another node or two that it was supposedly waiting to sync from.)

Another problem after resyncing.

2023-01-21T00:25:17Z ERROR: ValidateBlockSignature: No pubkeys verified for RingCT stake
2023-01-21T00:25:17Z *** Corrupt block found indicating potential hardware failure; shutting down

Currently I have TWO happily synced and staking devnet wallets [only].

@seanPhill
Copy link
Collaborator

seanPhill commented Jan 21, 2023

While syncing or resyncing we might need to look into this error too. (Much repeated!)
...
Currently I have TWO happily synced and staking devnet wallets [only].

WHEN a wallet resynced from snapshot or otherwise has several blocks forked it is failing to stop requesting the same block from the connect=192.168.0... peer with ERROR: ProcessNewBlock: AcceptBlock FAILED (bad-block-sig, PoS block signature not valid (code 16))(-1) even though I've already tried invalidateblock (a slightly earlier bad block) as the replacement for that block is rejected, while the perfectly happy forced-connected node that is synced and staking along with the other peer, is accumulating MILLIONS of banscore, as the connection is force, so it won't disconnect.

BUT, the cause of the third wallet forking immediately after starting up after loading the snapshot, without syncing from the running wallets, is that it was not a locked wallet, and it didn't wait to sync after starting!! ... Now this third wallet is locked and HAS synced and has no gazillion of errors accumulating in the debug log.

What I don't know yet is whether any of this was caused by anything less reliable in the code, or it was just an unlucky data corruption due to the leading wallets being on my power grid as it suffered multiple outages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Coin Type: RingCT Specifically related to RingCT transactions Component: Consensus Part of the core cryptocurrency consensus protocol Component: Core App Related to the application itself. Component: Miner Both PoW and PoS block creation Component: Wallet Relating to keystore, tx creation, and balance tracking Tag: PoS Related to Proof of Stake Tag: Transaction Creation Related to the transaction creation process. Tag: TransactionRecords The display of transaction information Tag: Validation Related to the validation of blocks and transactions. Tag: Waiting For Code Review Waiting for code review from a core developer Tag: Waiting For QA A pull review is waiting for QA to test the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants